Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2단계 - 자주 가는 음식점] - 초코(강다빈) 미션 제출합니다. #176

Merged
merged 53 commits into from
Mar 19, 2024

Conversation

00kang
Copy link
Member

@00kang 00kang commented Mar 18, 2024

안녕하세요 케빈, 🍫초코🍫입니다.
먼저 step1의 미션에서 구현한 웹 컴포넌트를 최대한 활용하고자 노력했으나, 이 방식을 고집한 결과 이번 미션의 요구사항을 다 구현해내지 못했다는 양해의 말씀드립니다..ㅠ

🏃‍♀️ 실행 방법

npm install

# 프로그램 실행
npm run start

# 테스트 실행
npm run test

🔗 배포 링크



📍 학습 목표

  • 어플리케이션을 컴포넌트 단위로 모듈화하여 개발
    • UI를 컴포넌트 단위로 생각하고 개발하는 연습
    • 재사용할 수 있는 컴포넌트를 고민해보기
  • 웹 UI 환경에서의 테스트 기초
    • 사용자 관점에서 중요하다고 생각하는 기능을 스스로 정의하고 E2E 테스트로 검증해보기
  • TypeScript의 기본 문법을 익히며 필요성을 경험


📂 폴더 구조

폴더명 설명
domain 도메인 로직, 상태(데이터)를 저장하는 도메인들을 관리
service 도메인과 뷰를 연결하는 모듈을 관리
view 뷰 관리
└ components 웹 컴포넌트들을 관리
└ imgs 뷰에서 사용되는 이미지 파일을 관리
└ styles 웹 컴포넌트에서 사용되는 css 파일을 관리

파일 설명

📜 파일 트리 보기
.
src
 ┣ domain
 ┃ ┗ RestaurantManager.ts
 ┣ service
 ┃ ┗ WebController.js
 ┣ view
 ┃ ┣ components
 ┃ ┃ ┣ GNB.js
 ┃ ┃ ┣ Modal.js
 ┃ ┃ ┣ RestaurantDetail.js
 ┃ ┃ ┣ RestaurantForm.js
 ┃ ┃ ┣ RestaurantItem.js
 ┃ ┃ ┣ RestaurantList.js
 ┃ ┃ ┣ Select.js
 ┃ ┃ ┗ Tab.js
 ┃ ┣ imgs
 ┃ ┃ ┣ add-button.png
 ┃ ┃ ┣ category-asian.png
 ┃ ┃ ┣ category-chinese.png
 ┃ ┃ ┣ category-etc.png
 ┃ ┃ ┣ category-japanese.png
 ┃ ┃ ┣ category-korean.png
 ┃ ┃ ┣ category-western.png
 ┃ ┃ ┣ favorite-icon-filled.png
 ┃ ┃ ┗ favorite-icon-lined.png
 ┃ ┣ styles
 ┃ ┃ ┣ global.css
 ┃ ┃ ┣ Gnb.css
 ┃ ┃ ┣ Modal.css
 ┃ ┃ ┣ RestaurantDetail.css
 ┃ ┃ ┣ RestaurantForm.css
 ┃ ┃ ┣ RestaurantList.css
 ┃ ┃ ┣ Select.css
 ┃ ┃ ┗ Tab.css
 ┃ ┗ WebView.js
 ┗ index.js

현재 진행 상황

  • 음식점 목록에서 즐겨찾기 기능 추가
  • 즐겨찾기 여부에 따라 탭 구분
  • 음식점 디테일 모달
    • 삭제하기
    • 닫기
    • 즐겨찾기 버튼
    • 링크 연결
  • E2E 테스트
    • 탭 이동
    • 음식점 정렬
    • 음식점 추가
    • 음식점 삭제

❓ 질문사항

app-modal에 restaurant-form을 넣은 구조와 동일하게 app-modal에 restaurant-detail을 넣어서 모달 컴포넌트를 재사용하고자 했습니다. 그런데 어떠한 이벤트 동작도 먹히지 않아서, 즐겨찾기, 삭제하기, 닫기 버튼이 동작하지 않고 있습니다. 저번주부터 주말 내내 고민해봤지만 해결하지 못해, 오늘 크루들과 여러 시도를 해봤음에도 해결책을 찾지 못했습니다. 저희가 내린 결론은 웹 컴포넌트를 사용하지 않는 코드 구조로 수정하는 걸로 결론을 내렸지만,,, 케빈의 의견은 어떤지 질문드립니다 😢

  • 레스토랑 추가 폼과 동일하게 코드 구조를 가지고 가는데 왜 레스토랑 추가 모달은 정상 작동(요소 접근, 이벤트 동작)하고 디테일 모달은 아무런 이벤트, 접근을 하지 못하는 걸까요?
  • 이 웹 컴포넌트를 활용하면서 해결할 수 있는 방법이 있을까요?

  • 추가로, 이렇게 막힐 때는 어떻게 돌파구를 찾아야 할까요? 여러 크루들이 제대로 작동이 안되니까 코드를 갈아엎었다고 하더라고요. 저는 코드를 갈아엎게 되면 이전의 노력들이 다 사라지는 것 같아서, 의미없는 활동이 되어버려서, 다시 시작해야 한다는 막막함이 두려워서, 여러 이유로 코드를 갈아 엎는 것에 반대하는 입장인데 케빈의 생각은 어떤가요~?

llqqssttyy and others added 30 commits March 5, 2024 14:31
- eslint
- prettier
- tsconfig(경로 별칭)
- package.json(typescript-eslint 설치)

Co-Authored-By: 00kang <[email protected]>
- 페이지가 로드됐을 때
- 필터를 변경할 때

Co-Authored-By: 00kang <[email protected]>
@00kang 00kang changed the base branch from main to 00kang March 18, 2024 09:07
@00kang
Copy link
Member Author

00kang commented Mar 19, 2024

이벤트 동작 등록 안되던 오류 해결

해결 방법 : RestaurantItem.js 내의 아래 코드 삭제

modalContainer.innerHTML = '';
modalContainer.appendChild(content);

modal 웹 컴포넌트에 넣는 detail container가 클릭되기를 원했으나 위에 방법처럼 innerHTML로 slot 자체를 덮어씌워서일까?
detail container가 제대로 렌더링되지 않았고, 이벤트 또한 동작하지 않았다.

아직 정확한 오류 원인 파악은 안됨

@JeongBin0227 JeongBin0227 self-assigned this Mar 19, 2024
@JeongBin0227
Copy link

JeongBin0227 commented Mar 19, 2024

image

이렇게 등록하면 이미지가 깨지는 이슈가 있네요

@JeongBin0227
Copy link

domain | 도메인 로직, 상태(데이터)를 저장하는 도메인들을 관리

이라는 기준으로 전체적인 코드를 봤을때,
도메인 로직에서 직접 로컬 스토리지에 접근하는 것은 일반적으로 좋지 않는 방법입니다. 도메인 로직이 데이터의 저장 방식에 대해 너무 디펜던시가 있으면 추후 다른 저장 방식으로 변경하고자 할 때 유연성을 떨어뜨립니다. 대신, 데이터 저장 및 로드 로직을 별도의 데이터 계층으로 분리하는 것은 어떨까요?

@JeongBin0227
Copy link

레스토랑 추가 폼과 동일하게 코드 구조를 가지고 가는데 왜 레스토랑 추가 모달은 정상 작동(요소 접근, 이벤트 동작)하고 디테일 모달은 아무런 이벤트, 접근을 하지 못하는 걸까요?

보통 이벤트 리스너가 달리지 않는 이유는 타이밍 이슈가 대부분 입니다. 웹 컴포넌트가 완전히 로드되고 초기화되기 전에 이벤트 리스너를 추가하거나 요소에 접근하려고 하면 문제가 발생하기 때문에 컴포넌트 생명주기를 확인해보시면 좋을거 같아요. (console.log 등을 이용하면 좋습니다.)

그리고 일부 버블링 캡처링 이슈도 있을수 있는데, 웹 컴포넌트의 이벤트가 바깥으로 전파되지 않거나 외부 이벤트가 컴포넌트 내부로 전파되지 않을 수 있습니다. 이 부분도 같이 확인해보면 좋을거 같아요

@JeongBin0227
Copy link

추가로, 이렇게 막힐 때는 어떻게 돌파구를 찾아야 할까요? 여러 크루들이 제대로 작동이 안되니까 코드를 갈아엎었다고 하더라고요. 저는 코드를 갈아엎게 되면 이전의 노력들이 다 사라지는 것 같아서, 의미없는 활동이 되어버려서, 다시 시작해야 한다는 막막함이 두려워서, 여러 이유로 코드를 갈아 엎는 것에 반대하는 입장인데 케빈의 생각은 어떤가요~?

코드를 갈아 엎기전에 문제 원인을 명확하게 파악 해보면 좋을거 같아요. 문제의 원인을 정확히 이해하고 있지 않은 상태에서 코드를 처음부터 다시 작성하는 것은 같은 문제를 반복할 수 있습니다.
즉 코드를 갈아엎는건 마지막 수단으로 남겨두고, 최대한 기존 코드에서 해결하려고 노력하되, 구조적인 문제나 설계상 이슈가 확실하면 갈아엎고, 문제 원인을 명확하게 파악이 되면 갈아 엎어도 어떤 구조에 기존 코드가 어디에 들어갈지 명확하게 알 수 있어서 시간을 훨씬 절약할수 있습니다.

Copy link

@JeongBin0227 JeongBin0227 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요! 초코

같이 구조에 대해 많이 얘기해보고 고민해봤는데, 1단계에 비해 구조가 많이 좋아진거 같아요. 그래서 어떠한 기능이 추가됐을때, 어느곳에서 처리해줘야할지도 명확해진거 같고 각각의 책임이 잘 나눠진거 같아요. 한가지 아쉬운점은 아무래도 시간이 부족해 완성도 측면이 조금 아쉬운데 이부분은 시간이 된다면 좀더 보완해보면 좋을거 같아요.
대부분 코드가 비슷해서 지금 남겨드린 코드는 코드 전반적으로 다 해당되는 부분이니까 코멘트를 먼저읽어보고 수정하는것을 추천드려요!

2단계 미션 하느라 고생많으셨습니다 👍 다음 단계 미션도 화이팅입니다 💯

@@ -1,4 +1,4 @@
<!DOCTYPE html>
<!doctype html>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기 수정하신 이유가 있으실까요?

Comment on lines +61 to +63
updatedLinkElement.addEventListener('click', (event) => {
event.stopPropagation();
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기 코드의 이유가 있을까요??

#updateFavoriteIcon(isFavorite) {
const favoriteIcon = this.querySelector('.restaurant-detail__favorite-button img');
const favorite = isFavorite !== undefined ? isFavorite : JSON.parse(localStorage.getItem('favorite')) || false;
favoriteIcon.src = favorite ? './favorite-icon-filled.png' : './favorite-icon-lined.png';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 제대로 경로를 못찾아서 이미지가 깨지는 이유겠네요

}

#handleClose() {
console.log('close');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인이 끝난 console.log는 지워주는게 좋습니다~

} else {
this.#updateRestaurantList(this.#webView.categoryFilter, this.#webView.sortingFilter);
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#updateRestaurantList, #updateFavoriteRestaurantList, #handleFormSubmitUpdateUI 메서드에서 UI 업데이트 로직이 중복되고 있습니다. 이러한 중복은 코드의 유지보수를 어렵게 만듭니다. 가능하다면, UI 업데이트를 처리하는 공통 함수를 만들어 코드 중복을 줄여보면 어떨까요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RestaurantManager의 메소드 실행 중 발생할 수 있는 예외나 에러는 어디서 처리될까요?

@JeongBin0227 JeongBin0227 merged commit 483cf1a into woowacourse:00kang Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants